Skip to content

Rewrite blocking support at runtime & add clang pass#18

Merged
lim123123123 merged 5 commits intoITMO-PTDC-Team:masterfrom
Kirillog:clang_pass
Jul 7, 2025
Merged

Rewrite blocking support at runtime & add clang pass#18
lim123123123 merged 5 commits intoITMO-PTDC-Team:masterfrom
Kirillog:clang_pass

Conversation

@Kirillog
Copy link
Copy Markdown
Contributor

@Kirillog Kirillog commented Mar 30, 2025

Comment thread runtime/include/verifying_macro.h Outdated
// Tell that the function need to be converted to the coroutine.
#define non_atomic attr(ltest_nonatomic)
// Tell that the function must not contain interleavings.
#define ___atomic attr(ltest_atomic)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

имена, начинающиеся с __ зарезервированы для нужд компиляторов (ссылка), объявлять свои макросы/переменные, начиная с __, - уб.

как на счет: ___atomic -> as_atomic/treat_as_atomic/consider_atomic?

Comment thread runtime/lib.cpp Outdated
bool CoroBase::IsReturned() const { return is_returned; }

extern "C" void CoroYield() {
if (!__yield) [[unlikely]] {
Copy link
Copy Markdown
Contributor

@dmitrii-artuhov dmitrii-artuhov Mar 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

лучше все места, где есть нелокальные переменные, начинающиеся с __, переименовать, чтобы нижних подчеркиваний в их начале не было

Comment thread verifying/CMakeLists.txt Outdated
add_executable(${target} __tmp_${source_name})
# apply clangpass to the ${source_name} file
add_custom_command(
OUTPUT ${CMAKE_CURRENT_SOURCE_DIR}/__tmp_${source_name}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
OUTPUT ${CMAKE_CURRENT_SOURCE_DIR}/__tmp_${source_name}
OUTPUT ${CMAKE_CURRENT_SOURCE_DIR}/${CLANG_TOOL_TMP_PREFIX}${source_name}

Comment thread verifying/CMakeLists.txt Outdated
function(verify_target target)
add_executable(${target} ${source_name})
if (APPLY_CLANG_TOOL)
add_executable(${target} __tmp_${source_name})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
add_executable(${target} __tmp_${source_name})
add_executable(${target} ${CLANG_TOOL_TMP_PREFIX}${source_name})

Comment thread verifying/CMakeLists.txt Outdated
TARGET ${target}
POST_BUILD
COMMAND ${CMAKE_COMMAND} -E echo "Removing temporary file '${CMAKE_CURRENT_SOURCE_DIR}/${CLANG_TOOL_TMP_PREFIX}${source_name}' generated after building ${target}"
COMMAND ${CMAKE_COMMAND} -E remove ${CMAKE_CURRENT_SOURCE_DIR}/__tmp_${source_name}
Copy link
Copy Markdown
Contributor

@dmitrii-artuhov dmitrii-artuhov Mar 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
COMMAND ${CMAKE_COMMAND} -E remove ${CMAKE_CURRENT_SOURCE_DIR}/__tmp_${source_name}
COMMAND ${CMAKE_COMMAND} -E remove ${CMAKE_CURRENT_SOURCE_DIR}/${CLANG_TOOL_TMP_PREFIX}${source_name}

что-то много где одно и то же встречается, можно переменную тут завести, чтобы покороче писать

dmitrii-artuhov added a commit to dmitrii-artuhov/lincheck-cpp that referenced this pull request Apr 1, 2025
@Kirillog Kirillog force-pushed the clang_pass branch 3 times, most recently from 9f72bc5 to 7071ce0 Compare April 13, 2025 21:03
Comment thread runtime/include/scheduler.h Outdated
for (size_t i = 0; i < tasks_in_thread; ++i) {
if (!IsTaskRemoved(thread[i]->GetId())) {
thread[i] = thread[i]->Restart(&state);
thread[i] = thread[i]->Restart(&*state);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

что за жесть)) почему не state.get()?)

@Kirillog Kirillog force-pushed the clang_pass branch 6 times, most recently from 54b8236 to 4fd6690 Compare April 23, 2025 18:50
@Kirillog Kirillog force-pushed the clang_pass branch 5 times, most recently from e785f08 to 5f840a3 Compare May 5, 2025 21:54
@Kirillog Kirillog marked this pull request as ready for review May 5, 2025 21:54
extend switch point insertion

fix hook & folly shared mutex

try to setup CI to run folly tests

refactor folly_rwspinlock

add bank && custom blocking primitives

format code

speed up futex blocking && change release schema

forbid CoroYield calls in inappropriate contexts

fix CI

add flatcombining queue

use renaming action

clang_pass: support simple names & several match types

refactor

enable CI

fix livelock problem && get rid of legacy token API

format code

fix pct backoff strategy

add ad-hoc termination order

refactor verifying targets

fix after rebasing onto minimization patch

refactor clang pass

rewrite totally blocking support at runtime

fix livelock avoiding

reformat code && enable testing

fix formatting scripts && reformat code

fix review comments

add conditional variables testing

add conditional variable primitive

fix futex_queues.PopAll

pct_strategy: run round robin for livelock avoding

add deadlock detection

refactor code

refactor StrategyVerifier

extend buffered channel test
Copy link
Copy Markdown
Collaborator

@lim123123123 lim123123123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Мне кажется, что стоит сделать один бинарь, который сразу запустит кланг плагин + рантайм
Что по итогу делаем со старым пассом?

Comment thread Dockerfile
RUN apt install -y libboost-filesystem-dev libboost-program-options-dev libboost-regex-dev \
libdouble-conversion-dev libfast-float-dev libevent-dev libssl-dev libfmt-dev \
libgoogle-glog-dev zlib1g-dev && \
git clone https://github.com/Kirillog/folly.git && \
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Мы теперь каждый раз будем фолли клонить?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кажется это чиниться или https://github.com/actions/cache или тем что мы зальем наш образ на условный docker hub и будем его клонить

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Потенциально еще и каждый раз userver пропатченный будем клонить и собирать, вообще хорошо образ на докерхаб залить, я согласен, можно донести это после защит

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я хз насколько хорошо этот кэш работает, ему по идее нужно на ноде это кэшировать, те на всех раннерах
Докер не поможет особо, только какой-то базовый, тк основной кейс - дебаг нашего кода, те будут правки в рантайме и тд

return EXIT_FAILURE;
}

// auto files = eOptParser->getSourcePathList();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

clang::tooling::RefactoringTool tool(options->getCompilations(),
options->getSourcePathList());

return tool.run(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я правильно понимаю, что предполагается использовать это как два бинаря? Сначала прогоняешь этот, потом загоняешь это в runtime?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nonlinear_queue.cpp -> (clang pass) __tmp_nonlinear_queue.cpp -> (clang++ -fpass-plugin=yieldPass.so) ./nonlinear_queue, который затем уже запускается. Объединить компиляцию с llvm пассом и clang pass нельзя, особенность libTooling API.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему нельзя? Ну сделай main, который сделает proc.Exec("clang++ aboba") + запустит потом что нужно

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В смысле предлагается сделать ltest_clang++, который сначала clang_pass запускает, а потом clang++ -fpass-plugin=yieldPass.so ... ? Честно говоря, clang_pass не настолько готов, чтобы его можно было свободно использовать как часть пайплайна clang-а, который потом будет компилировать большую библиотеку на 1000 таргетов наподобие userver или folly, я бы пока отдельным исполняемым файлом оставил.

Copy link
Copy Markdown
Collaborator

@lim123123123 lim123123123 May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Предлагается сделать на бинарь, который внутри просто будет вызывать кланг, в кланг это встраивать не нужно

Comment thread clangpass/include/clangpass.h
Comment thread runtime/include/pct_strategy.h Outdated
if (current_schedule_length == priority_change_points[i]) {
priorities[index_of_max] = current_depth - i;

if (round_robin_stage > 0) [[unlikely]] {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Мб это в Base вынести или хотя бы в функцию, чтобы не дублировать?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно вынести в функцию, которая будет зависеть от лямбды в качестве параметра (task_index), но это общая проблема копипасты в минимизации, ее надо комплексно решать

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ну давай просто тут вынесем хотя бы, я считаю, что проблему копипасты нужно решать по мере ее появления

Comment thread runtime/include/round_robin_strategy.h
Comment thread runtime/lib.cpp
for (size_t i = 0; i < 1000 && !is_returned; ++i) {
Resume();
}
} No newline at end of file
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Мб ассерт добавить в конец?

@lim123123123 lim123123123 merged commit 15f64f0 into ITMO-PTDC-Team:master Jul 7, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants